Conversation
|
Please run cargo fmt so CI is happy, looks good with that :) |
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
(Ticking request changes so I notice when it gets updated)
c811ec6 to
07c058c
Compare
emilio
left a comment
emilio
left a comment
There was a problem hiding this comment.
Hmm, I don't think ignoring them in miri is good... can we keep the include_str! in miri instead?
Thanks — updated so miri still uses include_str! for the JSON fixtures, while non-miri keeps runtime load for crates.io archives. Dropped the miri ignores on those tests. |
| "stylesheet_bytes.json" => include_str!("css-parsing-tests/stylesheet_bytes.json"), | ||
| "An+B.json" => include_str!("css-parsing-tests/An+B.json"), | ||
| "urange.json" => include_str!("css-parsing-tests/urange.json"), | ||
| other => panic!("unknown css-parsing-tests fixture: {other}"), |
There was a problem hiding this comment.
This seems unfortunate, let's just make it a macro like:
macro_rules! css_parsing_test_json {
($path:tt) {
#[cfg(miri)]
String::from(include_str!($path))
#[cfg(not(miri))]
std::fs::read_to_string(&format!("{}/src/{}", env!(..), $path)).unwrap()
}
}
Seems like that should work?
There was a problem hiding this comment.
Done — replaced the match helper with css_parsing_test_json! as suggested (miri: include_str!, otherwise fs::read_to_string via CARGO_MANIFEST_DIR). Pushed in 28f5c96.
|
@rome-xi could you squash your commits into one? |
Use css_parsing_test_json! so miri keeps include_str! while normal builds load fixtures from disk via CARGO_MANIFEST_DIR (crates.io archives omit include_str!-embedded paths).
28f5c96 to
5b063db
Compare
|
Squashed into a single commit ( |
2 participants
Closes #213.
Replace
include_str!for the CSS parsing JSON fixtures withstd::fs::read_to_stringsocargo testcompiles from the crates.io package (those files are inexclude). Missing fixtures still fail at runtime (no silent green); they can be--skipped when packaging.